Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Support TLS in Webternet. #1856

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open

Add Support TLS in Webternet. #1856

wants to merge 20 commits into from

Conversation

idk1703
Copy link

@idk1703 idk1703 commented Nov 4, 2022

Resolves #1497.

@idk1703
Update cURL version to 7.86.0.
382e571
@idk1703
Add Mbed TLS 3.2.1.
e460bc0
@idk1703
Update cURL configs.
a725c80 
Update config-sm-linux.h.
Update config-sm-mac.h. (mac supported now ?)
Add config-sm-win.h.
@idk1703
Update AMBuild scripts.
d540019 
Update Webternet AMBuilder.
Update cURL AMBuilder.
Add Mbed TLS AMBuilder.
@idk1703
Update cURL extension.
811608e 
Add path to Certificate Authority (CA) bundle.
Remove unused func UTIL_Format and UTIL_FormatArgs.
@idk1703
Add ca-bundle.crt.
d105d07 
From https://curl.se/docs/caextract.html.
@sapphonie
Copy link
Contributor

image
I'm no alliedmodders developer, but I would consider paring down your merge request a bit...

@idk1703
Copy link
Author

idk1703 commented Nov 4, 2022

изображение Я не разработчик alliedmodders, но я бы подумал о том, чтобы немного сократить ваш запрос на слияние...

I think so too, but I see the full version of cURL in repo.
Need delete all, except include, lib/library and license.

@rumblefrog
Copy link
Contributor

I think a git submodule would be appropriate.

@asherkin
Copy link
Member

asherkin commented Nov 4, 2022 •
edited
Loading

Thanks for working on this!

I'm no alliedmodders developer, but I would consider paring down your merge request a bit...

This is fine, the individual commits are suitably sized.

@idk1703
Move ca-bundle.crt.
8ec5a79 
Update PackageScript: remove copy not exist dir.
Update extension.h: change path ca-bundle.
@idk1703
Move custom files.
347e934 
For replace curl-src and mbedtls-src to submodules.
@idk1703
Remove curl-src and mbedtls-src.
d0319fe
@idk1703
Add cURL as submodule.
2c30db2
@idk1703
Add Mbed TLS as submodule.
e10171b
@idk1703
Update AMBuilder.
539bd2f 
Move custom files in submodules on build.
@idk1703
Copy link
Author

idk1703 commented Nov 5, 2022

I think this better, for keep clear repo.

@idk1703 idk1703 requested a review from asherkin November 5, 2022 16:04
asherkin
asherkin reviewed Nov 25, 2022
View reviewed changes
Copy link
Member

@asherkin asherkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks really good - great work, and nice to get a cURL update out of it too.

I think the deps are probably a question that'll take some debate - @alliedmodders/sourcemod how do we feel about introducing submodules for external dependencies? I was initially a little against it and preferred having things vendored in-tree for reliability, but especially given the provenance of these projects (and our other deps that could benefit from this treatment - sqlite and libpcre) and the fact we have SP submoduled so a recursive checkout is already required, keeping the repo cleaner is appealing.

extensions/curl/AMBuilder Outdated Show resolved Hide resolved
@psychonic
Copy link
Member

how do we feel about introducing submodules for external dependencies?

I am in favour of this.

asherkin
asherkin requested changes Nov 25, 2022
View reviewed changes
Copy link
Member

@asherkin asherkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One additional request to go with the AMBuild changes above, please could you add a feature provider for this so that exts can check if it is available? Take a look at the search results for AddCapabilityProvider for inspiration.

@idk1703 idk1703 requested a review from asherkin November 27, 2022 11:00
@idk1703 idk1703 removed the request for review from asherkin November 29, 2022 19:52
@idk1703 idk1703 requested a review from peace-maker November 29, 2022 19:52
@peace-maker
Copy link
Member

Symlinks on windows require special capabilitities or SYSTEM privileges. So this breaks casual builds on windows :(
@dvander knows AMBuild the best and probably knows how to integrate this best. Maybe we could get curl to look for the config files in other directories as well?

@dvander
Copy link
Member

dvander commented Dec 2, 2022

symlink isn't the only problem. (1) third-party links in submodules are risky. They can and do disappear. They should be cloned into the alliedmodders organization. (2) modifying the source tree during the build is asking for trouble. instead, once the clone is available, these can be checked in as prebuilts, and updated on downstream merges.

@idk1703
Move files to repos.
3547bc4 
Remove symlinks, now it's not need.
@idk1703
Copy link
Author

idk1703 commented Dec 15, 2022 •
edited
Loading

Need import or fork repos in AlliedModders.

curl, mbedtls

@idk1703
Copy link
Author

idk1703 commented Dec 18, 2022 •
edited
Loading

How create pr to new branch ? on curl and mbedtls repos.
sm-ext <- sm-ext
curl: sm-ext - new branch from tag curl-7_86_0
mbedtls: sm-ext - new branch from tag mbedtls-3.3.0

@KyleSanderson
Copy link
Member

@idk1703 are you still blocked on this?

@idk1703
Update
ab41a03 
Move submodules, use repos from AM org
Update AMBuilder
Add AMBuilder for curl and mbedtls libs
Add configs for curl
@idk1703
make submodules shallow
c0e5b0e
@idk1703
Copy link
Author

idk1703 commented Nov 30, 2023

So i think this done

curl - 7.86.0
mbedtls - 3.3.0

@peace-maker peace-maker mentioned this pull request Dec 11, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asherkin asherkin asherkin requested changes

@Frenzoid Frenzoid Frenzoid approved these changes

@peace-maker peace-maker Awaiting requested review from peace-maker

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support TLS in Webternet
9 participants
@idk1703 @sapphonie @rumblefrog @asherkin @psychonic @peace-maker @dvander @KyleSanderson @Frenzoid